app/subscription/core_templates.py (1)

7-10: Consider adding a TTL to @cached() as a safety net against stale data.

@cached() with no ttl argument means the cached value never expires — invalidation relies entirely on NATS messages reaching handle_core_template_message. If NATS delivery fails or is delayed, stale template data persists indefinitely.

Adding a modest TTL (e.g., 5–10 minutes) provides a safety net:

♻️ Suggested change

-@cached()
+@cached(ttl=300)
 async def subscription_core_templates() -> dict[str, str]:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/core_templates.py` around lines 7 - 10, The cached
subscription_core_templates function currently uses `@cached`() with no ttl so
values never expire; add a modest TTL (e.g., ttl=300 or ttl=600) to the `@cached`
decorator on subscription_core_templates to ensure the cache auto-invalidates
periodically as a safety net if NATS messages to handle_core_template_message
are missed or delayed, while keeping get_core_template_values and the existing
manual invalidation logic intact (use the same decorator signature where
subscription_core_templates is defined).

app/subscription/default_templates.py (1)

18-22: Hardcoded region-specific DNS servers in default template.

The proxy-server-nameserver entries (178.22.122.100, 78.157.42.100) are Shecan DNS servers specific to Iran. If this panel is intended for broader use, consider using more universally appropriate defaults (e.g., 1.1.1.1, 8.8.8.8) or documenting these as region-specific.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/default_templates.py` around lines 18 - 22, The default
template contains hardcoded region-specific DNS entries under the key
"proxy-server-nameserver" (values 178.22.122.100 and 78.157.42.100); update the
template in app/subscription/default_templates.py to use globally appropriate
defaults (for example 1.1.1.1 and 8.8.8.8) or replace these values with clearly
documented region-specific placeholders and add a comment/README note indicating
they are Iran-specific so operators can override them; locate the "nameserver"
and "proxy-server-nameserver" entries in the template and change the values or
add the documentation accordingly.

app/db/models.py (1)

717-729: Model looks well-structured.

The composite unique constraint and type index align with the query patterns in the CRUD layer. Using String(32) for template_type (instead of SQLEnum) defers validation to the application layer, which is flexible.

One optional observation: unlike most other models in this file (Admin, Node, CoreConfig, etc.), CoreTemplate lacks a created_at timestamp. If auditing template creation/modification times is useful, consider adding one.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/models.py` around lines 717 - 729, CoreTemplate is missing a
created_at timestamp—add a Mapped[datetime] mapped_column named created_at to
the CoreTemplate model (class CoreTemplate) so creation time is recorded;
implement it with a non-nullable DateTime column using a server default (e.g.,
func.now()) to auto-populate, and ensure you import DateTime and func from
sqlalchemy if not already present.

app/db/crud/core_template.py (6)

134-141: isinstance(s.value, tuple) branch is dead code.

CoreTemplateSortingOptionsSimple values are SQLAlchemy UnaryExpression objects (produced by .asc() / .desc()), never tuples. The sort_list.extend(s.value) path is unreachable and can be simplified away.

♻️ Proposed simplification

     if sort:
-        sort_list = []
-        for s in sort:
-            if isinstance(s.value, tuple):
-                sort_list.extend(s.value)
-            else:
-                sort_list.append(s.value)
-        stmt = stmt.order_by(*sort_list)
+        stmt = stmt.order_by(*[s.value for s in sort])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 134 - 141, The tuple branch is
dead because CoreTemplateSortingOptionsSimple items yield SQLAlchemy
UnaryExpression objects, not tuples; update the sorting logic in
core_template.py to always collect s.value (e.g., replace the isinstance branch
with a simple comprehension or single append per item) so sort_list becomes
[s.value for s in sort] and then call stmt = stmt.order_by(*sort_list);
reference symbols: sort, sort_list, s.value, and stmt.order_by.

---

134-141: Dead branch: isinstance(s.value, tuple) is never True.

CoreTemplateSortingOptionsSimple values are SQLAlchemy UnaryExpression objects (produced by .asc() / .desc()), never tuples. The sort_list.extend(s.value) path is unreachable and can be removed.

♻️ Proposed simplification

     if sort:
-        sort_list = []
-        for s in sort:
-            if isinstance(s.value, tuple):
-                sort_list.extend(s.value)
-            else:
-                sort_list.append(s.value)
-        stmt = stmt.order_by(*sort_list)
+        stmt = stmt.order_by(*[s.value for s in sort])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 134 - 141, The branch checking
isinstance(s.value, tuple) is dead because CoreTemplateSortingOptionsSimple
produces SQLAlchemy UnaryExpression objects (from .asc()/.desc()), so remove the
unreachable tuple branch and simplify the loop in the sort handling: build
sort_list by appending s.value for each s in sort and then call
stmt.order_by(*sort_list); update the block around sort, sort_list, s.value, and
stmt.order_by accordingly to eliminate the isinstance(...) branch.

---

62-64: Silent DB failure in get_core_template_values emits no log.

When the SELECT fails with SQLAlchemyError, the function silently falls back to hardcoded defaults with no warning. Operators would have no visibility that the database is unreachable and all subscribers are being served stale defaults.

♻️ Proposed fix — add a warning log

     except SQLAlchemyError:
+        logger.warning("Failed to fetch core template values from DB; falling back to defaults", exc_info=True)
         return defaults

(Import or reuse a module-level logger, e.g. logger = get_logger("core-template-crud"))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 62 - 64, The catch in
get_core_template_values currently swallows SQLAlchemyError and returns defaults
silently; update the function to log a warning with the exception details before
returning defaults by obtaining/using a module-level logger (e.g. define logger
= get_logger("core-template-crud") at top or reuse an existing logger) and
change the except block to log the error/stack (include the exception object)
and then return defaults so operators can see DB failures.

---

108-111: if limit: skips the limit clause when limit=0, returning all rows instead of none.

Using truthiness on an int parameter treats 0 as falsy. For offset=0 this is a no-op (harmless), but limit=0 silently drops the .limit(0) call, returning the full result set. The same pattern appears in get_core_templates_simple (lines 148–151).

♻️ Proposed fix (apply identically in get_core_templates_simple lines 148–151)

-    if offset:
+    if offset is not None:
         query = query.offset(offset)
-    if limit:
+    if limit is not None:
         query = query.limit(limit)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 108 - 111, The conditionals use
truthiness so limit=0 is treated as False and query.limit(0) is skipped; change
the checks to explicit None checks (e.g., replace "if limit:" with "if limit is
not None:" and similarly "if offset:" with "if offset is not None:") in the
function that builds the query and also apply the identical fix inside
get_core_templates_simple so query.limit(0) and query.offset(0) are honored.

---

62-64: Silent DB failure in get_core_template_values — no log emitted.

When the SELECT fails with SQLAlchemyError the function silently returns hardcoded defaults. Operators would have no indication that the DB is unreachable and templates have fallen back to defaults. A warning-level log before the return defaults would make this observable.

♻️ Proposed fix

+import logging
+_logger = logging.getLogger(__name__)
 ...
     except SQLAlchemyError:
+        _logger.warning("Failed to fetch core template values from DB; falling back to defaults", exc_info=True)
         return defaults

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 62 - 64, get_core_template_values
currently swallows SQLAlchemyError and returns defaults silently; update the
except block to log a warning before returning. Specifically, in
get_core_template_values catch SQLAlchemyError as e (or similar), call the
module logger (e.g., logger.warning or processLogger.warning) with a clear
message including the exception details and context (e.g., "Failed to fetch core
template values, falling back to defaults: %s" % e), then return defaults.
Ensure you reference SQLAlchemyError, the defaults variable, and the
get_core_template_values function when making the change.

---

108-111: Use is not None guard for offset and limit to avoid incorrect behavior with limit=0.

if offset: and if limit: treat 0 as falsy. For offset=0 this is harmless (no-op), but limit=0 silently skips the limit and returns all rows instead of zero. The same pattern repeats in get_core_templates_simple (lines 148–151).

♻️ Proposed fix (apply the same change in `get_core_templates_simple` lines 148–151)

-    if offset:
+    if offset is not None:
         query = query.offset(offset)
-    if limit:
+    if limit is not None:
         query = query.limit(limit)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/db/crud/core_template.py` around lines 108 - 111, Change the truthy
checks for offset and limit to explicit None checks so zero values aren't
treated as falsy: replace the "if offset:" and "if limit:" guards with "if
offset is not None:" and "if limit is not None:" in the function containing the
shown query modifications (and apply the same change in
get_core_templates_simple where the same pattern occurs) so a provided limit=0
correctly applies a zero-row limit.

app/models/core_template.py (2)

76-81: Inconsistent total-count field names across list response types.

CoreTemplateResponseList (line 77) calls the total count count, while CoreTemplatesSimpleResponse (line 94) calls it total. API consumers that handle both shapes will need special-case knowledge of which field to read.

♻️ Proposed alignment (unify to `total`)

 class CoreTemplateResponseList(BaseModel):
-    count: int
+    total: int
     templates: list[CoreTemplateResponse] = []

Update the construction in app/operation/core_template.py line 95:

-    return CoreTemplateResponseList(templates=templates, count=count)
+    return CoreTemplateResponseList(templates=templates, total=count)

Also applies to: 92-94

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/models/core_template.py` around lines 76 - 81, The list response uses
inconsistent total field names: change CoreTemplateResponseList.count to
CoreTemplateResponseList.total (type int) and update any construction/usages
that build instances (e.g., the response created in the code that constructs
CoreTemplateResponseList in the core template operations—previously setting
"count") to set "total" instead; ensure CoreTemplatesSimpleResponse and
CoreTemplateResponseList now share the same field name and adjust any
tests/serializations that read "count" to read "total" (leave model_config
as-is).

---

76-81: Inconsistent list-response field naming: count vs total.

CoreTemplateResponseList exposes the size field as count (line 77), while CoreTemplatesSimpleResponse calls the same concept total (line 94). API consumers that handle both response shapes will need to know which field name to use, which is error-prone.

♻️ Proposed alignment (pick one name consistently)

 class CoreTemplateResponseList(BaseModel):
-    count: int
+    total: int
     templates: list[CoreTemplateResponse] = []

     model_config = ConfigDict(from_attributes=True)

Then update the construction site in app/operation/core_template.py line 95:

-    return CoreTemplateResponseList(templates=templates, count=count)
+    return CoreTemplateResponseList(templates=templates, total=count)

Also applies to: 92-94

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/models/core_template.py` around lines 76 - 81, The list response fields
are inconsistent: CoreTemplateResponseList defines the size field as count while
CoreTemplatesSimpleResponse uses total; pick one name (e.g., total) and make
both models use it consistently by renaming the field in
CoreTemplateResponseList (replace count:int with total:int) and adjusting any
default/initialization (templates: list[CoreTemplateResponse] = []). Then update
the code that constructs these DTOs (the place that creates a
CoreTemplateResponseList instance in the core_template operation) to
pass/consume the chosen field name (total) instead of count so both response
shapes match.

tests/api/test_core_template.py (1)

7-26: Tests don't clean up created templates.

None of the test functions delete the templates they create. If the test database isn't reset between runs (or between tests in the same session), leftover templates could cause flaky results in other tests. Consider adding cleanup in a finally block or using a pytest fixture with teardown.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/api/test_core_template.py` around lines 7 - 26, The
test_core_template_create_and_get test creates a template via
create_core_template but never deletes it, risking test pollution; modify the
test to remove the created template in a teardown step (e.g., wrap the
create/assert logic in try/finally and call the API to delete the template by
created["id"] in the finally block) or move creation to a pytest fixture that
yields the created template and deletes it in the fixture teardown, ensuring
cleanup uses the same client/Authorization header used in client.get.

app/routers/core_template.py (3)

66-74: Unconventional route construction using "s" suffix.

The list endpoints are defined as @router.get("s") and @router.get("s/simple"), relying on the prefix /api/core_template to form /api/core_templates and /api/core_templates/simple. While functional, this is non-obvious and could confuse contributors. A more conventional approach would be to define the prefix as /api/core_templates and use "" / "/simple" for list routes, and "/{template_id}" for singular resource routes — or split into two routers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/routers/core_template.py` around lines 66 - 74, The route decorators use
an unconventional `"s"` suffix to build plural paths (e.g., `@router.get`("s") on
get_core_templates) which is confusing; update the router prefix to a plural
form (e.g., change the router prefix from "/api/core_template" to
"/api/core_templates") and then change the list route decorators to use "" and
"/simple" (e.g., `@router.get`("") for get_core_templates and
`@router.get`("/simple") for get_core_templates_simple) and ensure singular routes
use "/{template_id}" (e.g., get_core_template_by_id) so path construction is
explicit and readable.

---

84-84: Parameter all shadows the Python built-in.

Renaming to e.g. fetch_all or include_all avoids shadowing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/routers/core_template.py` at line 84, The parameter named "all" shadows
the Python built-in; rename it (e.g., to include_all or fetch_all) in the
function signature where "all: bool = False" appears (and update the parameter
annotation) and then update every usage inside that function and all call sites
to use the new name (also adjust any type hints, default handling, and tests or
docs referencing the old name). Ensure the new identifier is used consistently
in any related methods or templates that forwarded the parameter (search for
"all=" and "all :" occurrences) to avoid runtime NameError or shadowing issues.

---

56-63: Returning {} from a 204 No Content endpoint.

HTTP 204 semantics require no response body. Returning an empty dict is not idiomatic and may cause some clients or middleware to behave unexpectedly. Return None or use Response(status_code=204) instead.

♻️ Suggested fix

 `@router.delete`("/{template_id}", status_code=status.HTTP_204_NO_CONTENT)
 async def remove_core_template(
     template_id: int,
     db: AsyncSession = Depends(get_db),
     admin: AdminDetails = Depends(check_sudo_admin),
 ):
     await core_template_operator.remove_core_template(db, template_id, admin)
-    return {}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/routers/core_template.py` around lines 56 - 63, The delete endpoint
remove_core_template currently returns an empty dict which violates 204 No
Content semantics; update the function (remove_core_template in
app/routers/core_template.py) to not return a body — either return None or
explicitly return a fastapi.Response(status_code=status.HTTP_204_NO_CONTENT)
after calling core_template_operator.remove_core_template(db, template_id,
admin) so no response body is emitted.

app/subscription/clash.py (1)

31-31: Inconsistent None-check pattern: or vs is None.

base.py and singbox.py use if xxx is None: to default template content, but here or is used. With or, an empty string "" would also trigger the fallback (which may be desirable), but it's inconsistent with the sibling classes. Consider aligning to one pattern across all subscription classes for predictable behavior.

♻️ Suggested alignment with other classes

-        self.clash_template_content = clash_template_content or DEFAULT_CLASH_SUBSCRIPTION_TEMPLATE
+        if clash_template_content is None:
+            clash_template_content = DEFAULT_CLASH_SUBSCRIPTION_TEMPLATE
+        self.clash_template_content = clash_template_content

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/clash.py` at line 31, The current assignment uses
"self.clash_template_content = clash_template_content or
DEFAULT_CLASH_SUBSCRIPTION_TEMPLATE", which treats empty strings as missing and
differs from the "if ... is None:" pattern used in base.py and singbox.py;
change the logic to mirror those classes by checking "if clash_template_content
is None:" and only then set self.clash_template_content =
DEFAULT_CLASH_SUBSCRIPTION_TEMPLATE (otherwise assign the provided value) so
empty-string inputs remain preserved and behavior is consistent with other
subscription classes.

app/subscription/base.py (2)

37-37: Unnecessary explicit del of local variables.

user_agent_data and grpc_user_agent_data are local variables that will be garbage-collected when __init__ returns. The explicit del adds no value.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/base.py` at line 37, Remove the unnecessary explicit
deletion of local variables: delete the statement "del user_agent_data,
grpc_user_agent_data" from the __init__ method so that local variables
user_agent_data and grpc_user_agent_data are allowed to go out of scope
naturally and be garbage-collected without an explicit del.

---

24-30: No error handling around template rendering/parsing.

If a core template stored in the DB contains invalid Jinja syntax or malformed JSON, render_template_string or json.loads will raise an unhandled exception, breaking subscription generation for all users. While validation occurs at template creation time, consider wrapping these in a try/except that falls back to the default template and logs a warning, so a corrupt template doesn't take down the subscription endpoint.

🛡️ Suggested defensive fallback

+        import logging
+        logger = logging.getLogger(__name__)
+
         self.proxy_remarks = []
-        user_agent_data = json.loads(render_template_string(user_agent_template_content))
+        try:
+            user_agent_data = json.loads(render_template_string(user_agent_template_content))
+        except Exception:
+            logger.warning("Failed to render user-agent template, falling back to default")
+            user_agent_data = json.loads(render_template_string(DEFAULT_USER_AGENT_TEMPLATE))
         if "list" in user_agent_data and isinstance(user_agent_data["list"], list):
             self.user_agent_list = user_agent_data["list"]
         else:
             self.user_agent_list = []

-        grpc_user_agent_data = json.loads(render_template_string(grpc_user_agent_template_content))
+        try:
+            grpc_user_agent_data = json.loads(render_template_string(grpc_user_agent_template_content))
+        except Exception:
+            logger.warning("Failed to render gRPC user-agent template, falling back to default")
+            grpc_user_agent_data = json.loads(render_template_string(DEFAULT_GRPC_USER_AGENT_TEMPLATE))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/base.py` around lines 24 - 30, The template rendering and
JSON parsing calls (render_template_string + json.loads) for
user_agent_template_content and grpc_user_agent_template_content can raise and
crash subscription generation; wrap each render/json.loads block in a try/except
that catches Exception, logs a warning via the class logger (e.g.,
self.logger.warning or whatever logger is used in this class) including which
template failed and the exception, and fall back to safe defaults (set
self.user_agent_list = [] and grpc_user_agent_data = {} or a default template
result) so a corrupt template doesn't break the subscription endpoint; apply
this defensive pattern around both the user_agent and grpc_user_agent processing
in the same method.

app/subscription/share.py (2)

75-76: No error handling if subscription_core_templates() fails.

If the DB/cache call fails, the exception will propagate directly to the HTTP handler. Since this is a user-facing subscription endpoint, consider a try/except with a fallback to default templates so that subscription generation remains available even during transient DB issues.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/share.py` around lines 75 - 76, Wrap the call to
subscription_core_templates() in a try/except to catch any exceptions, log the
error (e.g., logging.exception or processLogger.error) and set a safe fallback
for core_templates (for example a default templates list or an empty set) so
that _build_subscription_config(config_format, core_templates) still runs and
returns a usable conf; ensure you reference subscription_core_templates(),
core_templates and _build_subscription_config in the change and do not let the
original exception propagate to the HTTP handler.

---

37-65: Hard-coded dict keys are fragile — consider using constants.

The template keys ("USER_AGENT_TEMPLATE", "GRPC_USER_AGENT_TEMPLATE", "XRAY_SUBSCRIPTION_TEMPLATE", "SINGBOX_SUBSCRIPTION_TEMPLATE", "CLASH_SUBSCRIPTION_TEMPLATE") are raw string literals. A typo in any of them will produce a KeyError at runtime with no compile-time safety. Define these as module-level constants (or reference an enum) so they're defined once and shared with subscription_core_templates().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/share.py` around lines 37 - 65, Replace the raw string
template keys in _build_subscription_config with module-level constants (e.g.,
USER_AGENT_TEMPLATE, GRPC_USER_AGENT_TEMPLATE, XRAY_SUBSCRIPTION_TEMPLATE,
SINGBOX_SUBSCRIPTION_TEMPLATE, CLASH_SUBSCRIPTION_TEMPLATE) and use those
constants when constructing common_kwargs and when passing template_*_content
into StandardLinks, ClashMetaConfiguration, SingBoxConfiguration,
XrayConfiguration, and OutlineConfiguration; also update the provider of
core_templates (subscription_core_templates) to populate and export the same
constants so both functions reference the single source of truth and avoid
KeyError from mistyped literal keys.

app/subscription/singbox.py (1)

19-31: Same lack of error handling as base.py for template rendering.

json.loads(render_template_string(singbox_template_content)) on Line 31 will raise an unhandled exception if the template content contains invalid Jinja or malformed JSON. Consider wrapping with a try/except that falls back to DEFAULT_SINGBOX_SUBSCRIPTION_TEMPLATE, consistent with the recommendation for base.py.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/subscription/singbox.py` around lines 19 - 31, Wrap the template
rendering and JSON parsing in SingboxSubscription.__init__ in a try/except
around the call to json.loads(render_template_string(singbox_template_content))
so template/Jinja/JSON errors are caught; on exception, fall back to parsing
DEFAULT_SINGBOX_SUBSCRIPTION_TEMPLATE and assign to self.config (and optionally
log the error), keeping render_template_string, json.loads,
DEFAULT_SINGBOX_SUBSCRIPTION_TEMPLATE and self.config as the referenced symbols
to locate the change.